I think that we should separate the code responsible for validating password to a separate method in the LocalAuthenticator class (right now it's a part of validate() method).

Then we should use this method in validation rule since in the future valid_password implementation may depend on the used authentication library. So, I guess we shouldn't tightly couple this rule with LocalAuthenticator.

Also... if this rule is intended only for currently logged in users, why not skip the parameter part and use user() helper method to grab user data inside the rule?

@michalsn You're right about not needing parameter so I've updated the code to get the user via auth helper user() method.

I've also added one additional rule valid_password_with which works in the same manner as required_with so I had this rules previously. Which also was impacted by a CI4 issue codeigniter4/CodeIgniter4#2953

permit_empty|required_with[newPassword]|valid_password[{id}]

Which can now be replaced with single rule

valid_password_with[newPassword]

I'like the idea of using LocalAuthenticator but splitting the password check will also require updating the AuthenticatorInterface. So it might be a tricky to do this properly. The only way that looks feasible to me is moving password check to method called

validate_password($entity, $password)

And then use is_callable on Authenticator to check if validate_password exists as it does not make sense to have validate_password in the interface as it won't be possible to implement it in any password less authentication.

I've also added one additional rule valid_password_with which works in the same manner as required_with so I had this rules previously.

My personal preference is that if something can be done with simple 2 rules, then adding another one to do the same is unnecessary. Because we simply end up with an additional code that we have to maintain.

I'like the idea of using LocalAuthenticator but splitting the password check will also require updating the AuthenticatorInterface. So it might be a tricky to do this properly.

Actually I can imagine validate_password() method in the AuthenticatorInterface. If this method were not supported by the "driver", then I guess it would have to return false (or even throw an exception) - simple as that. There is still no official release yet, so I guess we can do this kind of change?

Maybe Lonnie will have some opinion about it and idea how to handle this. Or maybe he will say to just leave it as it is :)

Sorry this one slipped by me. I agree with @michalsn here. I'm good adding that to the interface, but don't think valid_password_with is needed.

@najdanovicivan Are you interested in making these changes or can I close?

@lonnieezell I’m on it

I've done the changes to include validate_password method in the AuthenticatorInterface but now I have some additional thoughts

What about renaming the method to validate_credential($user, $credential) that way it will be more generic so it will be useable for example with session tokens

Now in valid password $authenticator is get with Service methods which have $authenticationLib set to local as default.

It might be a good idea to change rule name to valid_credential as well and add an optional parameter for the authenticationLib with default value of local

@lonnieezell @MGatner Can we see this one through so it can get merged. I did not get any response related to renaming the method to validate credential

@lonnieezell will have to weigh in on the name change.